Thanks you very much for tackling this!

I didn't look at the code of this PR but want to share some thoughts on the failure.

Here is the program that is run:

for arg in  %O %A %B %L %P %S %X %Y %F; do echo $arg >> \"%A\"; done; cat \"%O\" \"%B\" >> \"%A\""

The failing output has 10 lines, whereas a valid output has 11 lines. Probably that is related. My feeling is that one of the arguments in the for loop substitutions is empty. It would disappear then, and everything is offset.
If these were quoted that shouldn't happen and is probably a good idea just to make the test-script more robust.

From there it should become more obvious which parameter comes out empty, and from there it should become clear what the problem really is.

I hope that helps.

The failing output has 10 lines, whereas a valid output has 11 lines. Probably that is related.

Yes, in the failing run, the theirs line is absent, and one of the b lines is moved to the top.

A couple things didn't mention in #1862 (comment):

My feeling is that one of the arguments in the for loop substitutions is empty. It would disappear then, and everything is offset.

It doesn't look like that specific mechanism accounts for the failure, because in the failing case a b line appears as the first line of the output, while otherwise b does not appear until the fifth line.

As an early step (before even doing most of the testing described above), I tried adding ' ' quotes around the format specifiers and " " quotes around the parameter expansion in the merge driver command. The effect was more splitting. For example, ancestor label became two lines, ancestor and label. I think part of what using unquoted parameter expansion is currently achieving for the echo arguments is split on newlines, when then effectively get replaced with spaces because when echo is run with multiple arguments it joins them with spaces. I am very unclear on what the exact intended behavior of this merge driver test command is.

The broader issue for me examining this specific test is that I am familiar neither with the details of gix-merge and its tests nor (even though this is general Git knowledge) with the format expected for custom merge drivers.

Thanks you very much for tackling this!

No problem. My goal that this is related to is to be able to fix some other bugs where we do not run scripts correctly, such as bugs in the special casing of scripts with shebangs when use_shell is false. Fixing such bugs runs the risk of producing worse problems if there are other bugs whose effects are worse than the effect of existing bugs. (For example, running a wrong command, instead of failing to run anything, would often be worse. Likewise, running a wrong command in circumstances that arise more readily could be worse than running a wrong command in circumstances that arise less readily.) It also runs the risk that I introduce new bugs that weren't present at all before.

But that also means I risk become separated from the original bug-fixing goals, which this is peripheral to, if I shift my focus too much onto this. If possible, I'd like to avoid become too embroiled in the details of the gix-merge tests. It might not be possible to avoid this, but there are two other approaches I may attempt first to try to figure out what is going on:

• Write something to examine how the environment is changed before and after the changes here. Manual testing with similar but non-identical ways of running shell processes suggests that the environment changes are not very great on my local machine (which is where they would have to be significant in order to be the cause of the problems I am observing on that machine but not CI). But this has the advantage that (a) it might be readily adaptable into a regression test, and (b) if I can accurately state how the environments are different, then your much greater familiarity with the failing gix-merge test and the intended gix-merge behavior might make the problem obvious even if it is not obvious to me.
• Investigate failures on a conceptually related feature branch where I am trying to improve how gix-testtools finds bash. I'm not sure yet because this could be the result of a mistake (and unfortunately, the full test suite takes quite some time to run with GIX_TEST_IGNORE_ARCHIVES=1 on my local Windows 10 development machine), but it looks like there are comparable failures--and many of them, though most may be from a single failing fixture script--when I try to make changes there that harmonize how gix-testtools runs bash with how this PR tries to make gix-path find sh. The changes I'm working on in gix-testtools seem much smaller than those here, yet I have far worse local test failures, so that's interesting. I'll try to give more information on that soon, possibly in another draft PR about that.

Edit: Reworded for clarity and reordered the sections.

theirs is empty, even though it should be a be something that the caller knows.

I am very unclear on what the exact intended behavior of this merge driver test command is.

It attempts to make the arguments that the merge-driver was called with observable. I think there is no quoting because each of the arguments is known not to have spaces in it. However, the lack of quoting also makes empty arguments unobservable, a definitive shortcoming of the current implementation.

The broader issue for me examining this specific test is that I am familiar neither with the details of gix-merge and its tests nor (even though this is general Git knowledge) with the format expected for custom merge drivers.

Here is the docs, but the short version is that it's a script into which various parameters can be substituted into - this substitution is performed by the the caller of the merge-driver. That caller does a bare string substitution, without any care for quotes or whitespace in general.

I'd like to avoid become too embroiled in the details of the gix-merge tests.

I definitely wouldn't want you to suffer unnecessarily. The key for me to solve this certainly is to reproduce the error. For now, it doesn't make much sense to me either, unless… it's a substitution engine, what if one of the arguments that it substitutes also contains substitutable characters? Probably that's not what's happening here though, as % should be uncommon enough.

This is a patch that should tell exactly what's executed, and from there it would become more obvious what's really happening.

diff --git a/gix-merge/src/blob/platform/merge.rs b/gix-merge/src/blob/platform/merge.rs
index 058a30128..324742a00 100644
--- a/gix-merge/src/blob/platform/merge.rs
+++ b/gix-merge/src/blob/platform/merge.rs
@@ -211,7 +211,7 @@ pub(super) mod inner {
                     cmd.extend_from_slice(token);
                 }
 
-                Ok(merge::Command {
+                Ok(dbg!(merge::Command {
                     cmd: gix_command::prepare(gix_path::from_bstring(cmd))
                         .with_context(context)
                         .command_may_be_shell_script()
@@ -223,7 +223,7 @@ pub(super) mod inner {
                     current_path: ours_path,
                     ancestor: base_tmp,
                     other: theirs_tmp,
-                })
+                }))
             }
 
             /// Return the configured driver program for use with [`Self::prepare_external_driver()`], or `Err`

I hope that helps.

Thanks--you're right that the actual arguments that are passed are important information. I had glossed over this because it did not look like they differed between the passing and failing environment, but that doesn't mean they aren't needed to know what's going on.

The results, with the patch you gave that reveals the Command instance with dbg!, when running the test on this feature branch from PowerShell where it fails, are:

C:\Users\ek\source\repos\gitoxide [run-ci/consistent-sh ≡ +0 ~1 -0 ~]> cargo nextest run -p gix-merge with_external
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.46s
────────────
 Nextest run ID 42b38560-812f-4bcd-9548-97bb529b272d with nextest profile: default
    Starting 1 test across 2 binaries (19 tests skipped)
     Running [ 00:00:00] 0/1: 0 running, 0 passed, 0 skipped
        FAIL [   0.158s] gix-merge::merge blob::platform::merge::with_external
──── STDOUT:             gix-merge::merge blob::platform::merge::with_external

running 1 test
test blob::platform::merge::with_external ... FAILED

failures:

failures:
    blob::platform::merge::with_external

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 19 filtered out; finished in 0.15s

──── STDERR:             gix-merge::merge blob::platform::merge::with_external
[gix-merge\src\blob\platform\merge.rs:214:20] merge::Command
{
    cmd:
    gix_command::prepare(gix_path::from_bstring(cmd)).with_context(context).command_may_be_shell_script().stdin(Stdio::null()).stdout(Stdio::inherit()).stderr(Stdio::inherit()).into(),
    current: ours_tmp, current_path: ours_path, ancestor: base_tmp, other:
    theirs_tmp,
} = "C:/Users/ek/scoop/apps/git/2.48.1/usr/bin/sh.exe" "-c" "for arg in  C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmp8ULmDC C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpmHJGGo C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpLByBCI 7 \'b\' \'ancestor label\' \'current label\' \'other label\' %F; do echo $arg >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpmHJGGo\"; done; cat \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmp8ULmDC\" \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpLByBCI\" >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpmHJGGo\"" "--"

thread 'blob::platform::merge::with_external' panicked at gix-merge\tests\merge\blob\platform.rs:297:13:
b
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

  Cancelling due to test failure
────────────
     Summary [   0.167s] 1 test run: 0 passed, 1 failed, 19 skipped
        FAIL [   0.158s] gix-merge::merge blob::platform::merge::with_external
error: test run failed

That's a bit hard to read due to the way the Command sub-object formats, and it does not automatically show output in the case that the test passes. So I have also tested with this change instead:

diff --git a/gix-merge/src/blob/platform/merge.rs b/gix-merge/src/blob/platform/merge.rs
index 058a30128..39e27e51f 100644
--- a/gix-merge/src/blob/platform/merge.rs
+++ b/gix-merge/src/blob/platform/merge.rs
@@ -399,6 +399,7 @@ impl<'parent> PlatformRef<'parent> {
         match self.configured_driver() {
             Ok(driver) => {
                 let mut cmd = self.prepare_external_driver(driver.command.clone(), labels, context.clone())?;
+                panic!("program: {:?}\nargs: {:#?}", cmd.get_program(), cmd.get_args());
                 let status = cmd.status().map_err(|err| Error::SpawnExternalDriver {
                     cmd: format!("{:?}", cmd.cmd),
                     source: err,

Then, when run from PowerShell, the output included:

program: "C:/Users/ek/scoop/apps/git/2.48.1/usr/bin/sh.exe"
args: CommandArgs {
    inner: [
        Regular(
            "-c",
        ),
        Regular(
            "for arg in  C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpayn7BE C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpEvLyJo C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpCbHa1a 7 \'b\' \'ancestor label\' \'current label\' \'other label\' %F; do echo $arg >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpEvLyJo\"; done; cat \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpayn7BE\" \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpCbHa1a\" >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpEvLyJo\"",
        ),
        Regular(
            "--",
        ),
    ],
}

When run from Git Bash, where the test passes when allowed to proceed, the output included this instead:

program: "C:/Users/ek/scoop/apps/git/2.48.1/usr/bin/sh.exe"
args: CommandArgs {
    inner: [
        Regular(
            "-c",
        ),
        Regular(
            "for arg in  C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpfax7ph C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpcPJLm3 C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmp5fMWd7 7 \'b\' \'ancestor label\' \'current label\' \'other label\' %F; do echo $arg >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpcPJLm3\"; done; cat \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpfax7ph\" \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmp5fMWd7\" >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpcPJLm3\"",
        ),
        Regular(
            "--",
        ),
    ],
}

This is to say that only the randomly generated suffixes after .tmp in temporary filenames are different (and they would differ across separate identical runs, too).

On the main branch, in PowerShell, where the test also passes if allowed to proceed, the only differences besides those temporary files' names is the shell command:

program: "sh"
args: CommandArgs {
    inner: [
        Regular(
            "-c",
        ),
        Regular(
            "for arg in  C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpD7kDyR C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmppnvU83 C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpkYgWkr 7 \'b\' \'ancestor label\' \'current label\' \'other label\' %F; do echo $arg >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmppnvU83\"; done; cat \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpD7kDyR\" \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmpkYgWkr\" >> \"C:\\Users\\ek\\source\\repos\\gitoxide\\gix-merge\\.tmppnvU83\"",
        ),
        Regular(
            "--",
        ),
    ],
}

State from previous runs in this experiment is not likely to be a contributing factor; I ran gix clean -xde in between.

On main, sh is used rather than a full. That is looked up to C:\Users\ek\scoop\shims\sh.exe on this system, which is a scoop shim that delegates to C:\Users\ek\scoop\apps\git\current\bin\sh.exe, which is the Git for Windows bin\sh.exe shim that delegates to the Git for Windows usr\bin\sh.exe. That latter distinction--that is, whether the Git for Windows sh.exe is involved--seems to be an essential element here, because the failure goes away under this change:

diff --git a/gix-path/src/env/mod.rs b/gix-path/src/env/mod.rs
index 78e2da294..cd6203df7 100644
--- a/gix-path/src/env/mod.rs
+++ b/gix-path/src/env/mod.rs
@@ -51,7 +51,7 @@ pub fn shell() -> &'static OsStr {
                     // more readable messages, append literally with `/` separators. The path from
                     // `git --exec-path` will already have all `/` separators (and no trailing `/`)
                     // unless it was explicitly overridden to an unusual value via `GIT_EXEC_PATH`.
-                    raw_path.push("/usr/bin/sh.exe");
+                    raw_path.push("/bin/sh.exe");
                     raw_path
                 })
                 .filter(|raw_path| {

But I am reluctant to apply that change without understanding what it is about the effect of the Git for Windows shim that is helping, to know that this shim really is preferable rather than the test happening to hinge on a quirk that is different. One reason I am reluctant is that, under that change, if git is provided by the Git for Windows SDK, its sh.exe would not be found. If the non-shim Git for Windows sh.exe is usable but just moderately less preferred, then we could check for the shim first and then fall back to the non-shim. But I would want to understand the situation better before jumping into that, in case really neither is preferred, or in case the non-shim executable does not work well enough for our needs to be worth using directly.

My guess is that the relevant effect of the shim is in changing the environment, but I am not certain it refrains entirely from adjusting arguments. (I think the shim does not adjust arguments directly. But even if that is right, msys-2.0.dll affects interpretation of arguments--that is, what ends up in the argv that a program that uses that DLL as its libc uses--and does so in a way that is affected by some environment variables.)

I'll try to modify the command to report its arguments and environment in a way that they cannot be confused with anything else, such as by writing them to a file. However, since this will necessarily modifying the command or its environment, it is not certain to be successful at revealing the cause.

Edit: A possible contributing factor is that I also have some bin directories associated with a regular MSYS2 installation (separate from Git for Windows) in my PATH on Windows.

When the shim is used, two directories belonging to the MSYS2 installation that is associated with the running shell, /mingw64/bin and /usr/bin, as well as my per-user bin at /c/Users/ek/bin (which I think is less important in this case), are prepended, to take precedence.

The reason I say /mingw64/bin and /usr/bin belong to the MSYS2 installation associated with the running shell is that they are Unix-style paths that are automatically treated that way and translated based on that when calling native external commands.

C:\Users\ek\scoop\apps\git\2.48.1\usr\bin\sh.exe -c 'echo "$PATH" | tr ":" "\n" | sort' > ubp.txt
C:\Users\ek\scoop\apps\git\2.48.1\bin\sh.exe -c 'echo "$PATH" | tr ":" "\n" | sort' > bp.txt
git --no-pager diff --no-index ubp.txt bp.txt

Where the output of that last command is:

diff --git a/ubp.txt b/bp.txt
index 7390dca90..7190bd5b3 100644
--- a/ubp.txt
+++ b/bp.txt
@@ -1,3 +1,6 @@
+/mingw64/bin
+/usr/bin
+/c/Users/ek/bin
 /c/Users/ek/scoop/apps/pwsh/7.5.0
 /c/Program Files (x86)/VMware/VMware Workstation/bin
 /c/Windows/system32

Note that this experiment is not equivalent to checking the $0, $@, and the environment variables in an actual merge driver command, and I am not certain if this is the cause. Even if it is, I am not clear on how it is the cause.

This is very strange and puzzling.
I understand that the way the shell is called is the same no matter what, so the reason for the difference is in how the shell scripted merge-driver is interpreted.

And that's particularly strange as there isn't even any argument to interpret, the values are quite literally baked in. All I can think of is poking around in the script, maybe by starting to execute it directly. It's pretty clear what is invoked in the working and non-working case, so that could be generalized to invocations that are repeatable on the command-line directly. From there it can be reduced and probed to see what exactly is causing the observable difference (assuming it reproduces in the first place). If it doesn't, we'd know that some environment variable is affecting it as well.

And I say this without even implying that you spend more time on this, I am just sharing thoughts.